Add an animation to pane entrance/exit by zadjii-msft · Pull Request #7364 · microsoft/terminal

Mike Griese (zadjii-msft) · 2020-08-21T14:49:16Z

Adds an entrance animation when panes are created. This animation can be
disabled with the disableAnimations global setting.

Although the XAML animation documentation was pretty heavy on the do it
in XAML route, our panes are created pretty much entirely in code, so
we've got to create the animations in code as well.

200ms as the duration of the animation was picked super arbitrarily.
300ms felt too long, and 166ms felt like it was only visible for a
single frame.

Validation Steps Performed

Man have I been opening panes

Closes #1001
Closes #7366

…rformance. Let's do _better_

This doesn't work, so I'm reverting it. The grid layout is created first and foremost, so the first pane is already clipped to the smaller size, no matter what. So let's just not.

…animations-2020 # Conflicts: # src/cascadia/TerminalApp/pch.h

Michael Niksa (miniksa) · 2020-08-21T17:52:06Z

What about the reverse animation on close pane?

Mike Griese (zadjii-msft) · 2020-08-21T17:53:37Z

Michael Niksa (@miniksa)

What about the reverse animation on close pane?

That's #7366 😉

Michael Niksa (miniksa)

The code looks fine. I just had the one comment I left outside. I feel like the animations should be paired (in and out) before merge.

…animations-2020 # Conflicts: # src/cascadia/TerminalApp/GlobalAppSettings.cpp # src/cascadia/TerminalApp/GlobalAppSettings.h

Mike Griese (zadjii-msft) · 2020-08-26T17:22:05Z

Alright so regarding animating the closing of panes.

This will be extra tricky. The simplest way to do it seems like it would be to:

In _ChildCloseRoutine, setup and start the animation
then on the completion of the animation, call _CloseChild to reparent the remaining panes, their controls, etc.

But that won't actually work for us. If we leave the row/col definitions untouched for the parent when one of their children closes, then even if we make the other child bigger, the child only has half the parent to work with. So the child will get bigger, but still be clipped by the existing rol/col defs

Oh but what if we

Remove the closed pane's grid from the parent
add a dummy grid in its place
set the parent pane's row/col defs to be auto sized, so they're as big as the content
animate the remaining pane growing to 100% size
animate the dummy grid shrinking in size to 0%
pre-resize the remaining pane (& all descendants) to the full size they will have. We'll set their size manually here.
start the animation.
on completion:
- remove all the manually set sizes from the remaining pane (& descendants)
- call _CloseChild to move the content from the remaining pane into the parent (this should be seamless)

Alright so I was about to say "this is too hard for us to do now", but with the auto-sizing of the parent grid this could be possible.

Andrew Harper (aharpervc) · 2020-08-26T21:32:59Z

Does this feature honor the "Show animations in Windows" setting (which also powers prefers-reduced-motion in browsers)?

Mike Griese (zadjii-msft) · 2020-08-27T15:29:11Z

Michael Niksa (@miniksa) you wanted closing animations? You get closing animations

Mike Griese (zadjii-msft) · 2020-08-27T16:50:35Z

Does this feature honor the "Show animations in Windows" setting (which also powers prefers-reduced-motion in browsers)?

Weird, this doesn't. But the new tab menu flyout does.

But if you disable animations app-wide with AllowDependentAnimations(false), and enable animations in windows, then the Pane animation doesn't run (expected), but the new tab menu does animate (unexpected). Same behavior with the command palette - it animates even with AllowDependentAnimations(false)

I wonder why that is. I'm gonna ping some XAML folks internally for some answers on this.

Dustin L. Howett (DHowett)

I'd definitely like for us to follow the system API -- there was an e-mail about how we can do it, so I'm gonna block for that and my couple comments.

I won't be around on Friday or Monday. I give permission to clear my review if they are fixed. I sign off apart from that.

I understand why we want a separate setting. It does feel weird though.

Dustin L. Howett (DHowett) · 2020-08-28T01:03:00Z

+
+        // Create the dummy grid. This grid will be the one we actually animate,
+        // in the place of the closed pane.
+        Controls::Grid dummyGrid;


shouldn't this be a border? we'll never use its containment properties..

idk, nit

(Still curious tho)

I just went with a Grid because that's my mental go-to "I need a 2d space" control. Is there a real difference the way I'm using this?

…animations-2020 # Conflicts: # src/cascadia/TerminalApp/GlobalAppSettings.h

…animations-2020

Mike Griese (zadjii-msft) · 2020-09-29T12:03:39Z

Dustin L. Howett (@DHowett) Hey turns out I did the OS setting thing right before I went on leave, wanna take another look?

…animations-2020

Dustin L. Howett (DHowett)

Augh! Sorry Mike. I wrote up comments and NEVER HIT SUBMIT

Dustin L. Howett (DHowett) · 2020-10-06T17:24:57Z

+// 200ms was chosen because it's quick enough that it doesn't break your
+// flow, but not too quick to see
+static const int AnimationDurationInMilliseconds = 200;
+static const Duration AnimationDuration = DurationHelper::FromTimeSpan(winrt::Windows::Foundation::TimeSpan(std::chrono::milliseconds(AnimationDurationInMilliseconds)));


the annoying bit here is that this has to be constructed during llibrary initialization :(

Dustin L. Howett (DHowett) · 2020-10-06T17:45:36Z

+
+        // Create the dummy grid. This grid will be the one we actually animate,
+        // in the place of the closed pane.
+        Controls::Grid dummyGrid;


(Still curious tho)

Dustin L. Howett (DHowett) · 2020-10-09T21:52:14Z

/azp run

Dustin L. Howett (DHowett) · 2020-10-09T21:52:55Z

+
+        // Create the dummy grid. This grid will be the one we actually animate,
+        // in the place of the closed pane.
+        Controls::Grid dummyGrid;


Dustin L. Howett (DHowett) · 2020-10-09T21:56:55Z

COPIED FROM BODY

Yesterday, I day-of-learned about animation in XAML. The result?

Summary of the Pull Request

Adds an entrance animation when panes are created. This animation can be disabled with the disableAnimations global setting.

and in 60FPS:

(though, playing that back, I might have the gif sped up a tad bit...)

References

panes megathread: Scenario: Add support for panes #1000
Creates follow up tasks: Pane opening animation should not flash the default pane backdrop #7365

PR Checklist

Closes Panes should have an optional motion effect on entrance #1001, closes Panes should have an animation when they're closed, too #7366
I work here
Tests added/passed
[n/a] Requires documentation to be updated

Detailed Description of the Pull Request / Additional comments

Although the XAML animation documentation was pretty heavy on the do it in XAML route, our panes are created pretty much entirely in code, so we've got to create the animations in code as well.

200ms as the duration of the animation was picked super arbitrarily. 300ms felt too long, and 166ms felt like it was only visible for a single frame.

Validation Steps Performed

Man have I been opening panes

azure-pipelines · 2020-10-09T22:11:24Z

Azure Pipelines successfully started running 1 pipeline(s).

Deleted user (ghost) · 2020-10-09T22:22:10Z

Hello Dustin L. Howett (@DHowett)!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

Adds an entrance animation when panes are created. This animation can be disabled with the `disableAnimations` global setting. Although the XAML animation documentation was pretty heavy on the _do it in XAML_ route, our panes are created pretty much entirely in code, so we've got to create the animations in code as well. 200ms as the duration of the animation was picked _super_ arbitrarily. 300ms felt too long, and 166ms felt like it was only visible for a single frame. see also: * [Motion in practice](https://docs.microsoft.com/en-us/windows/uwp/design/motion/motion-in-practice) * [This example](https://docs.microsoft.com/en-us/uwp/api/Windows.UI.Xaml.Media.Animation.Storyboard?view=winrt-19041#examples) what what I ended up using, albeit ported to cppwinrt. * [`Timeline.AllowDependentAnimations`](https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.media.animation.timeline.allowdependentanimations?view=winrt-19041#Windows_UI_Xaml_Media_Animation_Timeline_AllowDependentAnimations) * [easing functions](https://docs.microsoft.com/en-us/windows/uwp/design/motion/key-frame-and-easing-function-animations#easing-functions) ## Validation Steps Performed Man have I been opening panes Closes microsoft#1001 Closes microsoft#7366

## Summary of the Pull Request Since we've started working on the Settings UI, a few settings have been added on `main`. This adds those missing settings over. ## References Missing settings include... - #7364: `disableAnimations` - #7873: `launchMode` `focus` and `maximizedFocus` - #7793: `bellStyle` ## Validation Steps Performed Verified that those settings appear properly in the Settings UI.

Deleted user (ghost) · 2020-11-11T18:12:59Z

🎉Windows Terminal Preview v1.5.3142.0 has been released which incorporates this pull request.:tada:

Handy links:

Mike Griese (zadjii-msft) added 9 commits August 20, 2020 10:46

Move the dll project to /dll and move the lib project up

ebf75ed

Huh, this definitely creates an animation, but it's got HORRIFYING pe…

cb468e6

…rformance. Let's do _better_

animating one split > animating 0 splits

22a8b4d

hot damn this is so C R I S P

d7e872c

Add a bunch of code cleanup

d472fe5

Try animating both the first and second panes

dcbba94

This doesn't work, so I'm reverting it. The grid layout is created first and foremost, so the first pane is already clipped to the smaller size, no matter what. So let's just not.

Pane.cpp is ready for review, just gotta add the setting now

a09ec80

add a setting to disable animations, ready for review

522ab15

Merge remote-tracking branch 'origin/master' into dev/migrie/f/panes-…

f48fbfe

…animations-2020 # Conflicts: # src/cascadia/TerminalApp/pch.h

ghost added Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal. labels Aug 21, 2020

Mike Griese (zadjii-msft) changed the title ~~Add a aniamtion to pane entrance~~ Add a animation to pane entrance Aug 21, 2020

This was referenced Aug 21, 2020

Pane opening animation should not flash the default pane backdrop #7365

Open

Panes should have an animation when they're closed, too #7366

Closed

okay bot

188c969

Michael Niksa (miniksa) reviewed Aug 21, 2020

View reviewed changes

Anurag Thakur (AnuthaDev) reviewed Aug 21, 2020

View reviewed changes

Comment thread doc/cascadia/profiles.schema.json Resolved

Merge remote-tracking branch 'origin/master' into dev/migrie/f/panes-…

facc672

…animations-2020 # Conflicts: # src/cascadia/TerminalApp/GlobalAppSettings.cpp # src/cascadia/TerminalApp/GlobalAppSettings.h

holy shit it finally worked

fde5dca

Update this to look more crisp

38ddc35

Clean up this code for review

ae04952

Mike Griese (zadjii-msft) assigned Michael Niksa (miniksa) Aug 27, 2020

ghost added the Issue-Task It's a feature request, but it doesn't really need a major design. label Aug 27, 2020

Dustin L. Howett (DHowett) requested changes Aug 28, 2020

View reviewed changes

ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Aug 28, 2020

Mike Griese (zadjii-msft) added 2 commits August 28, 2020 11:30

Merge remote-tracking branch 'origin/master' into dev/migrie/f/panes-…

3c7d53f

…animations-2020 # Conflicts: # src/cascadia/TerminalApp/GlobalAppSettings.h

disable this animation when animations are disabled in the OS

cddf16c

ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Aug 28, 2020

Mike Griese (zadjii-msft) added 2 commits September 29, 2020 07:02

spellcheck nits

994d7e0

Merge remote-tracking branch 'origin/master' into dev/migrie/f/panes-…

984a309

…animations-2020

Mike Griese (zadjii-msft) assigned Dustin L. Howett (DHowett) Sep 29, 2020

Mike Griese (zadjii-msft) added 3 commits October 5, 2020 08:52

Merge remote-tracking branch 'origin/master' into dev/migrie/f/panes-…

d95e5c4

…animations-2020

Some merge conflicts fromt the month I was awaty

1653fa3

Merge remote-tracking branch 'origin/master' into dev/migrie/f/panes-…

5f09776

…animations-2020

Dustin L. Howett (DHowett) reviewed Oct 9, 2020

View reviewed changes

Dustin L. Howett (DHowett) approved these changes Oct 9, 2020

View reviewed changes

Dustin L. Howett (DHowett) changed the title ~~Add a animation to pane entrance~~ Add an animation to pane entrance/exit Oct 9, 2020

do something to make the azp run

082be75

Dustin L. Howett (DHowett) added the AutoMerge Marked for automatic merge by the bot when requirements are met label Oct 9, 2020

Dustin L. Howett (DHowett) removed their assignment Oct 9, 2020

ghost merged commit 9dc38ad into master Oct 9, 2020

ghost deleted the dev/migrie/f/panes-animations-2020 branch October 9, 2020 23:06

Carlos Zamora (carlos-zamora) mentioned this pull request Oct 30, 2020

Add missing settings to Settings UI #8116

Merged

ghost mentioned this pull request Nov 11, 2020

Panes should have an optional motion effect on entrance #1001

Closed

Mike Griese (zadjii-msft) mentioned this pull request Mar 4, 2024

Add SearchBox entry/exit animations #16808

Merged

This pull request was closed.

Conversation

Mike Griese (zadjii-msft) commented Aug 21, 2020 • edited by DHowett Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Validation Steps Performed

Uh oh!

Michael Niksa (miniksa) commented Aug 21, 2020

Uh oh!

Mike Griese (zadjii-msft) commented Aug 21, 2020

Uh oh!

Michael Niksa (miniksa) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Mike Griese (zadjii-msft) commented Aug 26, 2020

Uh oh!

Andrew Harper (aharpervc) commented Aug 26, 2020

Uh oh!

Mike Griese (zadjii-msft) commented Aug 27, 2020

Uh oh!

Mike Griese (zadjii-msft) commented Aug 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dustin L. Howett (DHowett) left a comment

Choose a reason for hiding this comment

Uh oh!

Dustin L. Howett (DHowett) Aug 28, 2020

Choose a reason for hiding this comment

Uh oh!

Dustin L. Howett (DHowett) Oct 6, 2020

Choose a reason for hiding this comment

Uh oh!

Mike Griese (zadjii-msft) Oct 9, 2020

Choose a reason for hiding this comment

Uh oh!

Dustin L. Howett (DHowett) Oct 9, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Mike Griese (zadjii-msft) commented Sep 29, 2020

Uh oh!

Dustin L. Howett (DHowett) left a comment

Choose a reason for hiding this comment

Uh oh!

Dustin L. Howett (DHowett) Oct 6, 2020

Choose a reason for hiding this comment

Uh oh!

Dustin L. Howett (DHowett) Oct 6, 2020

Choose a reason for hiding this comment

Uh oh!

Dustin L. Howett (DHowett) commented Oct 9, 2020

Uh oh!

Dustin L. Howett (DHowett) Oct 9, 2020

Choose a reason for hiding this comment

Uh oh!

Dustin L. Howett (DHowett) commented Oct 9, 2020

Summary of the Pull Request

References

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Man have I been opening panes

Uh oh!

azure-pipelines Bot commented Oct 9, 2020

Uh oh!

Deleted user (ghost) commented Oct 9, 2020

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

Uh oh!

Deleted user (ghost) commented Nov 11, 2020

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Mike Griese (zadjii-msft) commented Aug 21, 2020 •

edited by DHowett

Loading

Mike Griese (zadjii-msft) commented Aug 27, 2020 •

edited

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.